Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use multi-value with interface types #1764

Merged
merged 10 commits into from
Sep 16, 2019
Merged

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Sep 11, 2019

No description provided.

This crate provides a transformation to turn exported functions that use a
return pointer into exported functions that use multi-value.

Consider the following function:

```rust
pub extern "C" fn pair(a: u32, b: u32) -> [u32; 2] {
    [a, b]
}
```

LLVM will by default compile this down into the following Wasm:

```wasm
(func $pair (param i32 i32 i32)
  local.get 0
  local.get 2
  i32.store offset=4
  local.get 0
  local.get 1
  i32.store)
```

What's happening here is that the function is not directly returning the
pair at all, but instead the first `i32` parameter is a pointer to some
scratch space, and the return value is written into the scratch space. LLVM
does this because it doesn't yet have support for multi-value Wasm, and so
it only knows how to return a single value at a time.

Ideally, with multi-value, what we would like instead is this:

```wasm
(func $pair (param i32 i32) (result i32 i32)
  local.get 0
  local.get 1)
```

However, that's not what this transformation does at the moment. This
transformation is a little simpler than mutating existing functions to
produce a multi-value result, instead it introduces new functions that wrap
the original function and translate the return pointer to multi-value
results in this wrapper function.

With our running example, we end up with this:

```wasm
;; The original function.
(func $pair (param i32 i32 i32)
  local.get 0
  local.get 2
  i32.store offset=4
  local.get 0
  local.get 1
  i32.store)

(func $pairWrapper (param i32 i32) (result i32 i32)
  ;; Our return pointer that points to the scratch space we are allocating
  ;; on the shadow stack for calling `$pair`.
  (local i32)

  ;; Allocate space on the shadow stack for the result.
  global.get $shadowStackPointer
  i32.const 8
  i32.sub
  local.tee 2
  global.set $shadowStackPointer

  ;; Call `$pair` with our allocated shadow stack space for its results.
  local.get 2
  local.get 0
  local.get 1
  call $pair

  ;; Copy the return values from the shadow stack to the wasm stack.
  local.get 2
  i32.load
  local.get 2 offset=4
  i32.load

  ;; Finally, restore the shadow stack pointer.
  local.get 2
  i32.const 8
  i32.add
  global.set $shadowStackPointer)
```

This `$pairWrapper` function is what we actually end up exporting instead of
`$pair`.
Copy link
Contributor

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! The transform itself is quite pristine and looks fantastic, and I've mostly just got some nits about the integration here and there.

While you're working on this, mind prototyping the support necessary to integrate this into wasmtime as well?

crates/cli-support/src/lib.rs Show resolved Hide resolved
crates/cli-support/src/lib.rs Outdated Show resolved Hide resolved
crates/cli-support/src/webidl/standard.rs Outdated Show resolved Hide resolved
crates/multi-value-xform/src/lib.rs Show resolved Hide resolved
@fitzgen
Copy link
Member Author

fitzgen commented Sep 12, 2019

While you're working on this, mind prototyping the support necessary to integrate this into wasmtime as well?

My hope was to get this landed default off with an env var to enable it, and then get wasmtime working with it, and then enable it by default in interface types mode. How does that sound?

@fitzgen
Copy link
Member Author

fitzgen commented Sep 12, 2019

@alexcrichton any idea what's up with these CI failures related to sccache?

Starting sccache server...
+ env
++ pwd
+ SCCACHE_ERROR_LOG=/home/vsts/work/1/s/sccache.log
+ RUST_LOG=debug
+ /home/vsts/work/1/s/sccache-0.2.10-x86_64-unknown-linux-musl/sccache --start-server
DEBUG 2019-09-11T00:34:45Z: sccache::config: Attempting to read config file at "/home/vsts/.config/sccache/config"
DEBUG 2019-09-11T00:34:45Z: sccache::config: Couldn't open config file: No such file or directory (os error 2)
DEBUG 2019-09-11T00:34:45Z: sccache::config: Attempting to read config file at "/home/vsts/.config/sccache/config"
DEBUG 2019-09-11T00:34:45Z: sccache::config: Couldn't open config file: No such file or directory (os error 2)
DEBUG 2019-09-11T00:34:50Z: tokio_reactor: dropping I/O source: 0
error: Timed out waiting for server startup
##[error]Bash exited with code '2'.

@alexcrichton
Copy link
Contributor

How does that sound?

👍

any idea what's up with these CI failures related to sccache?

Hm no idea, would assume spurious,but if it keeps coming back feel free to disable sccache and I can deal with it.

This tiny crate provides utilities for working with Wasm codegen
conventions (typically established by LLVM or lld) such as getting the shadow
stack pointer.

It also de-duplicates all the places in the codebase where we were implementing
these conventions in one-off ways.
It is failing to install / setup on CI.
@fitzgen fitzgen merged commit 04c9b32 into rustwasm:master Sep 16, 2019
@fitzgen fitzgen deleted the multi-value-xform branch September 16, 2019 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants